feat(pr-review): server inbox, honest recents, and stale-submit recovery (w5a) - #5362
Merged
Conversation
Add , a server-fixed GraphQL search over review-requested open PRs, so the mobile inbox reads only PRs the caller's token can see. drops non-PR and identity-less nodes instead of throwing, and the schema guard now validates the inbox query.
Start the pulls, repos, and GraphQL legs of getPullRequest, and the two paginate legs of listChecks, before awaiting. Await with Promise.allSettled and rethrow the first rejection in leg order, so a second failure never becomes an unhandled rejection. Authorization stays inside the token retry.
Route context expansion through the react-query cache so re-expanding a gap is a hit, and hold the parsed viewed-files map in a process cache that a generation guard keeps from being overwritten by a late first read. Sign-out still drops the private content.
Replace the entry screen's ScrollView with a single FlashList whose header and footer stay mounted in every inbox state, so direct URL open and Recents survive an inbox failure. Recents now write only after the first authorized payload, and failed entries expose Retry and Remove.
Partition queued comments by head SHA and send only the fresh ones, so a stale item never blocks the valid ones. Stale items stay queued with an Outdated badge and edit/delete, and removeComments carries the hydration guard so an in-flight loadDraft cannot re-add just-posted comments.
Replace the ScrollView with a FlashList and page on scroll, so a large PR does not mount every file row. A search still fetches the full listed set, and a failed next page shows a retry CTA.
Replace the members ScrollView with a FlashList and drive it from a pure item builder, with loading/error/empty evaluated in that order so an org error never renders No members yet. Extract the discussion tab's state selection into a pure, tested selector.
Use the repo's deferred-promise pattern (undefined as unknown as) and mockReturnValueOnce so the two late-read tests pass oxlint.
The mapper receives only fresh items; stale items are never sent.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryReviewed the incremental commits since f30037c (empty-approve no-op, fetch-to-completion error routing, viewed-file clear fence, and anti-slop type/lint follow-ups); previous reviewer findings are fixed and no new high-confidence issues remain. Files Reviewed (14 files)
Previous Review Summaries (3 snapshots, latest commit f30037c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit f30037c)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit c48dcae)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit dddb570)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (47 files)
Reviewed by grok-4.6 · Input: 276.1K · Output: 21.1K · Cached: 717.6K Review guidance: REVIEW.md from base branch |
Remove the identity error-kind mapping in selectPrInboxView and inline the one-line shouldLoadAllFiles predicate at its single call site.
iscekic
changed the title
iscekic
changed the title
pandemicsyn
approved these changes
Aug 19, 2026
pandemicsyn
left a comment
pandemicsyn
left a comment
Contributor
There was a problem hiding this comment.
lgtm, couple bot notes inline and some merge conflicts but preapproved
# Conflicts: # apps/mobile/src/lib/pr-review/diff/use-pr-diff-context-loader.ts # apps/mobile/src/lib/pr-review/pending-review-provider.tsx # apps/mobile/src/lib/pr-review/recent-prs.ts
Empty removeComments is a no-op so APPROVE before hydration cannot clear a persisted draft. Fetch-to-completion now surfaces a resolved page error so the search retry can render. clearViewedFiles fences the cache before deletion so a concurrent read never returns the prior account.
Anti-slop now rejects anonymous object return types, so the discussion, org-list, context-window, and partition selectors use named contracts. toNotificationHref casts the built path into the typed-route union so kilo-app typecheck passes after the main merge.
…s as strings check-unused rejected the unused DiscussionTabViewKind export. kilo-app lint rejected the NotificationHref cast because it did not change the type. Return the built path as a string instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reviewer Notes
The PR review entry screen now shows a server-backed inbox of pull requests requesting the viewer's review, paginated through a new
listInboxquery. The search string is fixed server-side, so a caller can only ever see pull requests their own GitHub token can already see. Direct URL open is unchanged, and pagination is capped at 20 pages to stay inside GraphQL search rate limits.Files
apps/web/src/lib/github-pr-review/mappers.ts— addsbuildInboxResultand theGraphQlInboxNodetype; drops non-PR search nodes.apps/web/src/lib/github-pr-review/dtos.ts— adds the inbox item and result schemas plusINBOX_PAGE_SIZE.apps/mobile/src/lib/pr-review/use-pr-inbox.ts— infinite-query hook; 20-page cap and 30-second stale time.apps/mobile/src/components/pr-review/pr-review-inbox-list.tsx— FlashList body composing the paste header, inbox rows, and recents footer.apps/mobile/src/components/pr-review/pr-review-inbox-view.ts— pure state selector for the seven inbox states.apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx— mounts the inbox list; renders the recents Retry and Remove buttons.apps/mobile/src/components/ui/icons.ts— adds the Inbox icon.The overview and check queries now start their sub-requests in parallel and settle them with
Promise.allSettled, rethrowing the first rejection in leg order. This removes the sequential round-trips that previously serialized the overview's REST and GraphQL legs and the checks' two paginations.Files
apps/web/src/routers/github-pr-review-router.ts— addslistInbox,INBOX_QUERY, andINBOX_SEARCH_QUERY; parallelizesgetPullRequestandlistChecks.The submit sheet now partitions queued comments into fresh and stale by comparing each comment's recorded commit SHA against the current head. Only fresh comments are sent in the batched submit; stale comments stay queued, are flagged outdated, and the sheet reports a partial result instead of dismissing. This replaces the previous behavior of submitting everything against the latest head and surfacing per-item 422s.
Files
apps/mobile/src/lib/pr-review/partition-pending-items.ts— splits the queue into fresh and stale; an empty head makes every item fresh.apps/mobile/src/components/pr-review/pr-review-submit.tsx— submits fresh items only, removes them viaremoveComments, and shows the partial result.apps/mobile/src/components/pr-review/pr-review-submit-view.ts— CTA label and partial-result copy selectors.apps/mobile/src/components/pr-review/pr-review-pending-comment-row.tsx— stale flag, Outdated badge, and updated hint copy.apps/mobile/src/lib/pr-review/build-submit-review-input.ts— documents the fresh-only submission contract.apps/mobile/src/lib/pr-review/pending-review-provider.tsx— addsremoveCommentswith the hydration-generation guard.Recent PRs now carry a last-load outcome and are written only after the first authorized payload loads; a failed open marks an existing entry failed rather than creating one. The entry screen shows a Retry button for failed entries and a Remove button for every entry.
Files
apps/mobile/src/lib/pr-review/recent-prs.ts— addslastResult,removeRecentPr, andmarkRecentPrFailed.apps/mobile/src/components/pr-review/pr-review-screen.tsx— backfills withlastResult: 'ok'; marks an existing entry failed on error.apps/mobile/src/components/pr-review/recent-pr-row-state.ts— row-state selector for title, identity, and failed marker.The file navigator now renders file rows in a virtualized list and loads pages on scroll instead of fetching the full file set on mount. An active search still drives fetch-to-completion so filtering covers the full listed set. The memoized rows read the latest select and toggle handlers through refs, so the rows stay identity-stable across a search re-render yet never go stale when a head-SHA change swaps the toggle handler.
Files
apps/mobile/src/components/pr-review/diff/pr-diff-file-navigator.tsx— FlashList with onEndReached and memoized rows; per-path callbacks read the latest handlers through refs.apps/mobile/src/lib/pr-review/diff/navigator-file-filter.ts— extracted path filter and load-all decision.The organization Members screen now renders a single virtualized list built from a pure item builder, with loading, error, and empty states selected by a dedicated error-view selector. Invited members now render alongside active members in the same list.
Files
apps/mobile/src/components/organization/members-screen.tsx— FlashList with section, member, and invite items.apps/mobile/src/components/organization/members-list-items.ts— item builder with exact composition rules.apps/mobile/src/components/organization/org-list-error-view.ts— error-view selector; no reconnect state.Diff context expansion now fetches through the React Query cache with a 5-minute stale time and 10-minute GC time instead of a raw tRPC client call, so repeated context loads reuse cached lines. Viewed-file state adds a process-lifetime cache with a generation guard; when a read observes a generation change, it returns the authoritative current map rather than the stale parse, so a toggle or clear never races a re-read.
Files
apps/mobile/src/lib/pr-review/diff/use-pr-diff-context-loader.ts—queryClient.fetchQuerywith stale and GC times.apps/mobile/src/lib/pr-review/diff/context-window.ts— extracted window arithmetic.apps/mobile/src/lib/pr-review/viewed-files.ts— cached map plus a generation guard; the guard now protects the caller's return value, not only the cache.The Discussion tab's state selection is extracted into a pure
selectDiscussionTabViewfunction so the seven outcomes can be unit-tested. The tab renders exactly what it rendered before; only the decision is moved.Files
apps/mobile/src/components/pr-review/pr-review-discussion-tab-view.ts— pure state selector for the seven tab states.apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx— uses the selector instead of inline branches.Satisfied on main, not reassigned and not reimplemented:
P1-F-45,P2-GH-52d,P1-F-46c,P2-E-30a,P2-E-33b.Two assumptions were corrected during the build:
submitReviewonexpectedHeadSha, and it does not returnstale_headin the review path;stale_headexists only in the merge reconcile path. Stale handling is solved entirely on the client.Tests: 20 test files added or updated.
Generated: none.
Verification
Visual Changes
PR Review entry screen with the server-backed inbox
Submit sheet showing an outdated comment kept in the queue
Human steps: none known.
Notes: none.